Change Criteria UI by zigzagdev · Pull Request #123 · zigzagdev/world-heritage-frontend

zigzagdev · 2025-12-16T05:53:49Z

No description provided.

…scription feat: create criteria json file

feat: create criteria domain & UI

Change Display Criteria UI

zigzagdev

OK

Copilot

Pull request overview

This pull request introduces a new interactive UI for displaying UNESCO World Heritage criteria. It adds localized criteria definitions, creates a reusable CriteriaTags component, and updates the heritage detail pages to use the new criteria display instead of plain text.

Key changes:

New domain model (criteria.ts) with localization support for UNESCO World Heritage criteria (i-x)
New CriteriaTags component that displays clickable criteria badges linking to UNESCO documentation
Updated heritage detail components to integrate the new criteria UI with locale-aware rendering

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`client/src/shared/uis/CriteriaTags.tsx`	New component that renders clickable criteria badges with titles and descriptions
`client/src/locals/ja/world-heritage-criteria.json`	Japanese localization for all 10 UNESCO criteria
`client/src/locals/en/world-heritage-criteria.json`	English localization for all 10 UNESCO criteria
`client/src/domain/criteria.ts`	Domain logic for criteria types, locales, and helper functions to retrieve localized criteria
`client/src/app/features/top/components/heritage-detail/heritage-detail.css`	New CSS classes for layout, sidebar, facts card, and map placeholder
`client/src/app/features/top/components/heritage-detail/HeritageSidebar.tsx`	Updated to accept locale prop and display formatted criteria inline
`client/src/app/features/top/components/heritage-detail/HeritageOverviewSection.tsx`	Replaced text criteria with CriteriaTags component, improved accessibility with aria-labelledby
`client/src/app/features/top/components/heritage-detail/HeritageHero.tsx`	Replaced category chip with CriteriaTags component
`client/src/app/features/top/components/heritage-detail/HeritageDetailLayout.tsx`	Added locale prop threading to child components

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-16T06:10:14Z

+        <div className="heritage-detail__chips">
+          {item.criteria?.length ? (
+            <CriteriaTags criteria={item.criteria} locale={locale} />
+          ) : (
+            <span className="heritage-detail__muted">—</span>
+          )}
+        </div>


The CSS classes 'heritage-detail__chips' and 'heritage-detail__muted' are used but not defined in any CSS file. These classes need to be added to the heritage-detail.css file or the elements should use existing classes. Without these styles, the criteria tags and the fallback dash may not be displayed correctly.

Copilot · 2025-12-16T06:10:14Z

+      <section className="heritage-detail__subsection" aria-labelledby="heritage-criteria-heading">
+        <h3 id="heritage-criteria-heading" className="heritage-detail__subsection-title">
+          Criteria
+        </h3>

-      <div className="heritage-detail__section">
-        <h3 className="heritage-detail__subsection-title">Criteria</h3>
-        <p className="heritage-detail__text">{item.criteriaText || "—"}</p>
-      </div>
+        <div className="heritage-detail__chips">
+          {item.criteria?.length ? (
+            <CriteriaTags criteria={item.criteria} locale={locale} />
+          ) : (
+            <span className="heritage-detail__muted">—</span>
+          )}
+        </div>
+      </section>
+
+      <section className="heritage-detail__subsection" aria-labelledby="heritage-area-heading">
+        <h3 id="heritage-area-heading" className="heritage-detail__subsection-title">
+          Area &amp; Buffer zone
+        </h3>

-      <div className="heritage-detail__section">
-        <h3 className="heritage-detail__subsection-title">Area &amp; Buffer zone</h3>
        <HeritageMetadataList
          items={[
-            { label: "Area", value: item.areaText },
-            { label: "Buffer zone", value: item.bufferText },
+            { label: "Area", value: item.areaText ?? "—" },
+            { label: "Buffer zone", value: item.bufferText ?? "—" },
          ]}
        />
-      </div>
+      </section>

      {item.isEndangered && (
-        <div className="heritage-detail__section heritage-detail__section--danger">
-          <h3 className="heritage-detail__subsection-title">Danger status</h3>
+        <section
+          className="heritage-detail__subsection heritage-detail__subsection--danger"
+          aria-labelledby="heritage-danger-heading"
+        >
+          <h3 id="heritage-danger-heading" className="heritage-detail__subsection-title">
+            Danger status
+          </h3>
          <p className="heritage-detail__danger-text">
            This property is inscribed on the List of World Heritage in Danger.
          </p>
-        </div>
+        </section>


The CSS classes 'heritage-detail__subsection' and 'heritage-detail__subsection--danger' are used but not defined in the CSS file. These classes should be added to heritage-detail.css to properly style these sections, or the existing 'heritage-detail__section' and 'heritage-detail__section--danger' classes should be used instead.

Copilot · 2025-12-16T06:10:15Z

          </a>
        )}
      </div>
+      <div className="heritage-detail__map-card" aria-label="Map">


The aria-label attribute on a div element has no semantic meaning without an appropriate ARIA role. Consider changing this div to a section element or adding role="region" to make the aria-label effective for screen readers.

Suggested change

<div className="heritage-detail__map-card" aria-label="Map">

<div className="heritage-detail__map-card" aria-label="Map" role="region">

Copilot · 2025-12-16T06:10:15Z

 import "./heritage-detail.css";
 import { HeritageMetadataList } from "./HeritageMetadataList";
+import { CriteriaTags } from "@shared/uis/CriteriaTags.tsx";
+import type { Locale } from "../../../../../domain/criteria.ts";


TypeScript imports should omit .tsx/.ts extensions in non-test runtime code files. The codebase convention is to use imports without extensions for relative paths. Remove the .tsx and .ts extensions from these import statements.

Suggested change

import type { Locale } from "../../../../../domain/criteria.ts";

import type { Locale } from "../../../../../domain/criteria";

Copilot · 2025-12-16T06:10:15Z

 import { HeroImage } from "./HeroImage";
 import "./heritage-detail.css";
+import { CriteriaTags } from "@shared/uis/CriteriaTags.tsx";
+import type { Locale } from "../../../../../domain/criteria.ts";


TypeScript imports should omit .tsx/.ts extensions in non-test runtime code files. The codebase convention is to use imports without extensions for relative paths. Remove the .tsx and .ts extensions from these import statements.

Suggested change

import type { Locale } from "../../../../../domain/criteria.ts";

import type { Locale } from "../../../../../domain/criteria";

Copilot · 2025-12-16T06:10:15Z

 import { HeritageOverviewSection } from "./HeritageOverviewSection.tsx";
 import { HeritageSidebar } from "./HeritageSidebar.tsx";
 import { HeritageGallery } from "./HeritageGallery";
+import type { Locale } from "../../../../../domain/criteria.ts";


TypeScript imports should omit .tsx/.ts extensions in non-test runtime code files. The codebase convention is to use imports without extensions for relative paths. Remove the .tsx and .ts extensions from these import statements.

Suggested change

import type { Locale } from "../../../../../domain/criteria.ts";

import type { Locale } from "../../../../../domain/criteria";

Copilot · 2025-12-16T06:10:16Z

+const formatCriteriaInline = (criteria: string[] | undefined) =>
+  criteria?.length ? criteria.map((c) => `(${c})`).join("") : "—";
+
 export function HeritageSidebar({ item }: Props) {


The locale prop is defined in the Props type but not destructured in the function signature. The function signature should be: export function HeritageSidebar({ item, locale }: Props) to match the Props type. Currently the locale parameter is accepted but never accessible in the function body.

Suggested change

export function HeritageSidebar({ item }: Props) {

export function HeritageSidebar({ item, locale }: Props) {

Copilot · 2025-12-16T06:10:16Z

+  const safeLocale: Locale = (LOCALES as readonly string[]).includes(locale)
+    ? (locale as Locale)
+    : "en";
+  return CRITERIA_BY_LOCALE[safeLocale][code];


The getCriteriaSafe function doesn't include the same fallback logic as the getCriteria function. If the specified locale doesn't contain the requested code, this will return undefined instead of falling back to English or a default value. Consider adding the same fallback chain as getCriteria (lines 36-43) to ensure consistent behavior.

Suggested change

return CRITERIA_BY_LOCALE[safeLocale][code];

const entry = CRITERIA_BY_LOCALE[safeLocale]?.[code];

if (entry) return entry;

const fallback = CRITERIA_BY_LOCALE.en?.[code];

if (fallback) return fallback;

return { title: code, description: "" };

zigzagdev added 8 commits December 14, 2025 17:36

feat: create criteria json file

0c37cb0

Merge pull request #120 from zigzagdev/feat/create/unesco-criteria-de…

4da44cf

…scription feat: create criteria json file

feat: create criteria domain & UI

e52f4b7

fix: delete not using comments

e03ef92

Merge pull request #121 from zigzagdev/feat/create-criteria-declare

234b7aa

feat: create criteria domain & UI

feat: criteria content changed

b737ae6

fix: change criteria display UI

e657bc9

Merge pull request #122 from zigzagdev/feat/display-changed-criteria-ui

f2c3d3d

Change Display Criteria UI

zigzagdev requested a review from Copilot December 16, 2025 05:53

zigzagdev self-assigned this Dec 16, 2025

zigzagdev linked an issue Dec 16, 2025 that may be closed by this pull request

Improve Criteria representation for better UX and clarity #116

Closed

Copilot started reviewing on behalf of zigzagdev December 16, 2025 05:54

View session

fix: failed CI test

058ed04

zigzagdev commented Dec 16, 2025

View reviewed changes

zigzagdev merged commit 06cd886 into main Dec 16, 2025
1 check passed

zigzagdev deleted the refactor/criteria-ui branch December 16, 2025 06:07

Copilot AI reviewed Dec 16, 2025

View reviewed changes

Navigation Menu

Repository navigation

Change Criteria UI - #123

Change Criteria UI#123
zigzagdev merged 9 commits into
mainfrom
refactor/criteria-ui

zigzagdev commented Dec 16, 2025

Uh oh!

zigzagdev left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 16, 2025

Uh oh!

Copilot AI Dec 16, 2025

Uh oh!

Copilot AI Dec 16, 2025

Uh oh!

Copilot AI Dec 16, 2025

Uh oh!

Copilot AI Dec 16, 2025

Uh oh!

Copilot AI Dec 16, 2025

Uh oh!

Copilot AI Dec 16, 2025

Uh oh!

Copilot AI Dec 16, 2025

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	<div className="heritage-detail__map-card" aria-label="Map">
	<div className="heritage-detail__map-card" aria-label="Map" role="region">

	import type { Locale } from "../../../../../domain/criteria.ts";
	import type { Locale } from "../../../../../domain/criteria";

	export function HeritageSidebar({ item }: Props) {
	export function HeritageSidebar({ item, locale }: Props) {

-  return CRITERIA_BY_LOCALE[safeLocale][code];
+  const entry = CRITERIA_BY_LOCALE[safeLocale]?.[code];
+  if (entry) return entry;
+  const fallback = CRITERIA_BY_LOCALE.en?.[code];
+  if (fallback) return fallback;
+  return { title: code, description: "" };

Conversation

zigzagdev commented Dec 16, 2025

Uh oh!

zigzagdev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants